-
Notifications
You must be signed in to change notification settings - Fork 113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversion webhook #1302
Conversion webhook #1302
Conversation
69ae8dd
to
1299ebf
Compare
ad5ac86
to
50c1881
Compare
04fcfb3
to
88bd994
Compare
eeaeb3d
to
af245c1
Compare
42c1b08
to
7aff8b7
Compare
d8f2aad
to
c7322d6
Compare
55dfc96
to
3f2443e
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adambkaplan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@qu1queee It looks like this pull request needs a rebase before we can merge it. |
if brStep.SecurityContext != nil { | ||
step.SecurityContext = brStep.SecurityContext | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need the nil check here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cant recall, isn't needed, same for the BuildStrategySpec.SecurityContext validation.
Modify the v1beta1 CR types
38398ac
to
fd85ac1
Compare
@adambkaplan I replied to your feedback, I took the liberty to resolve all conversations where I satisfied your request. Pls take a look. |
Add deployment artifacts for the webhook Add additional hack scripts, needed for patching the CRDs and generating a CABundle
Introduce cmd/shipwright-build-webhook Introduce conversion pkg
Add Build CR conversion functions
For build, buildruns.
fd85ac1
to
29fe106
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a few more smaller comments (such as not logging the admissionresponse on info but on debug level), but we can address those separately.
/lgtm
Changes
Build
,BuildRun
,BuildStrategy
andClusterBuildStrategy
.Fixes #1104
Submitter Checklist
Release Notes